-
Notifications
You must be signed in to change notification settings - Fork 2k
enhancement(observability): Add _utilization_mean buffer metrics
#24453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The source and transform buffer utilization metrics currently measure either the instantaneous level or a histogram, neither of which is a reliable single-valued reflection of the actual status. For this purpose, we introduce a `mean` metric that uses the EWMA calculations to produce a simple moving average that will better track the actual value.
maycmlee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of small suggestions
thomasqueirozb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
| let mut result = f64::NAN; | ||
| self.average | ||
| .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current| { | ||
| let average = if current.is_nan() { | ||
| point | ||
| } else { | ||
| point.mul_add(self.alpha, current * (1.0 - self.alpha)) | ||
| }; | ||
| result = average; | ||
| average | ||
| }); | ||
| result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| let mut result = f64::NAN; | |
| self.average | |
| .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current| { | |
| let average = if current.is_nan() { | |
| point | |
| } else { | |
| point.mul_add(self.alpha, current * (1.0 - self.alpha)) | |
| }; | |
| result = average; | |
| average | |
| }); | |
| result | |
| self.average | |
| .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current| { | |
| if current.is_nan() { | |
| point | |
| } else { | |
| point.mul_add(self.alpha, current * (1.0 - self.alpha)) | |
| } | |
| }) |
I think this would work the same as fetch_update returns an f64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that fetch_update returns the old value (hence the mnemonics of fetch-then-update), but we want to return the updated average which is only accessible in the closure or by adding another fetch which adds to the expense.
Summary
The source and transform buffer utilization metrics currently measure either the
instantaneous level or a histogram, neither of which is a reliable single-valued
reflection of the actual status. For this purpose, we introduce a
meanmetricthat uses the EWMA calculations to produce a simple moving average that will
better track the actual value.
Vector configuration
How did you test this PR?
Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook, please see this template.make fmtmake check-clippy(if there are failures it's possible some of them can be fixed withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details here.